feat(client): add opt-in card validation hooks#1142
Conversation
Introduce composable CardValidator hooks on ClientFactory and create_client, plus built-in helpers for HTTPS-only and private URL checks. Default behavior remains unchanged when no validators are set. Fixes a2aproject#975
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces optional validation hooks for AgentCard instances before client creation, including validators to reject non-HTTPS URLs, private/loopback IP addresses, and cards without supported interfaces. The feedback recommends strengthening the private URL check to explicitly reject hostnames like localhost and .local domains, extending URL extraction to include the provider.url field, and parameterizing the tests to cover these additional private and loopback address formats.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| host = parsed.hostname | ||
| if host is None: | ||
| raise InvalidAgentCardError(f'URL has no hostname: {url!r}') | ||
| try: | ||
| ip = ipaddress.ip_address(host) | ||
| except ValueError: | ||
| return |
There was a problem hiding this comment.
The _check_not_private function checks if the hostname is a private IP address, but it does not reject standard loopback or local hostnames like localhost or .local domains. Since ipaddress.ip_address raises a ValueError for non-IP hostnames, names like localhost or service.local will bypass this check. Explicitly rejecting localhost, .localhost, and .local domains prevents private network access via these hostnames.
| host = parsed.hostname | |
| if host is None: | |
| raise InvalidAgentCardError(f'URL has no hostname: {url!r}') | |
| try: | |
| ip = ipaddress.ip_address(host) | |
| except ValueError: | |
| return | |
| host = parsed.hostname | |
| if host is None: | |
| raise InvalidAgentCardError(f'URL has no hostname: {url!r}') | |
| host_lower = host.lower() | |
| if ( | |
| host_lower == 'localhost' | |
| or host_lower.endswith('.localhost') | |
| or host_lower.endswith('.local') | |
| ): | |
| raise InvalidAgentCardError( | |
| f'URL points to a private or loopback address: {url!r}' | |
| ) | |
| try: | |
| ip = ipaddress.ip_address(host) | |
| except ValueError: | |
| return |
| def _iter_card_urls(card: AgentCard) -> list[str]: | ||
| urls = [ | ||
| interface.url | ||
| for interface in card.supported_interfaces | ||
| if interface.url | ||
| ] | ||
| if card.documentation_url: | ||
| urls.append(card.documentation_url) | ||
| if card.icon_url: | ||
| urls.append(card.icon_url) | ||
| return urls |
There was a problem hiding this comment.
The _iter_card_urls helper extracts URLs from the card's supported interfaces, documentation, and icon, but it misses the provider.url field. If an untrusted card specifies a malicious or private URL in provider.url, it will bypass validation. Including card.provider.url ensures all metadata URLs are validated. Note that we only check and append card.provider.url if it is present, to handle cases where the URL might not be set.
| def _iter_card_urls(card: AgentCard) -> list[str]: | |
| urls = [ | |
| interface.url | |
| for interface in card.supported_interfaces | |
| if interface.url | |
| ] | |
| if card.documentation_url: | |
| urls.append(card.documentation_url) | |
| if card.icon_url: | |
| urls.append(card.icon_url) | |
| return urls | |
| def _iter_card_urls(card: AgentCard) -> list[str]: | |
| urls = [ | |
| interface.url | |
| for interface in card.supported_interfaces | |
| if interface.url | |
| ] | |
| if card.documentation_url: | |
| urls.append(card.documentation_url) | |
| if card.icon_url: | |
| urls.append(card.icon_url) | |
| if card.provider.url: | |
| urls.append(card.provider.url) | |
| return urls |
References
- When displaying provider information, ensure that the URL is only shown if it is present, even if the specification indicates it should be a required field, to handle cases where the current implementation might allow creation without the URL set.
- If a field in a data model (e.g.,
ServerCallContext.user) is non-optional, avoid adding redundant checks for its existence and rely on the data model's contract.
| def test_reject_private_urls_rejects_loopback(): | ||
| card = _valid_card( | ||
| supported_interfaces=[ | ||
| AgentInterface( | ||
| protocol_binding=TransportProtocol.JSONRPC, | ||
| url='https://127.0.0.1', | ||
| ) | ||
| ] | ||
| ) | ||
|
|
||
| with pytest.raises(InvalidAgentCardError, match='private or loopback'): | ||
| reject_private_urls(card) |
There was a problem hiding this comment.
Parameterize the private URL rejection test to cover localhost, .localhost, .local domains, and IPv6 loopback addresses ([::1]) to ensure comprehensive validation coverage.
@pytest.mark.parametrize(
'url',
[
'https://127.0.0.1',
'https://localhost',
'https://test.localhost',
'https://my-service.local',
'https://[::1]',
],
)
def test_reject_private_urls_rejects_loopback(url):
card = _valid_card(
supported_interfaces=[
AgentInterface(
protocol_binding=TransportProtocol.JSONRPC,
url=url,
)
]
)
with pytest.raises(InvalidAgentCardError, match='private or loopback'):
reject_private_urls(card)
🧪 Code Coverage (vs
|
| Base | PR | Delta | |
|---|---|---|---|
| src/a2a/client/client_factory.py | 88.67% | 88.82% | 🟢 +0.15% |
| src/a2a/client/card_validators.py (new) | — | 61.29% | — |
| Total | 92.97% | 92.75% | 🔴 -0.22% |
Generated by coverage-comment.yml
|
Hi @piyushbag, thank you for your PR, please note that the issue is labeled with maintainers-only, we're not accepting external contributions for it at this point. |
2 participants
Summary
a2a.client.card_validatorswith composableCardValidatorhooks andInvalidAgentCardError.ClientFactoryandcreate_client(default: no validation, backward compatible).require_supported_interfaces,reject_non_https_urls, andreject_private_urls.Problem
Untrusted cards resolved via
create_from_urlhad no SDK hook for caller-defined validation before client creation (#975). Callers needing HTTPS-only or private-network policies had to duplicate checks.Test plan
uv run ruff checkon changed filesuv run pytest tests/client/test_card_validators.py tests/client/test_client_factory.py -qFixes #975